More descriptive PR message when multiple dependencies are fixed in a security update#5595
More descriptive PR message when multiple dependencies are fixed in a security update#5595
Conversation
Nishnha
force-pushed
the
nishnha/transitive-pr-message
branch
2 times, most recently
from
7999e73 to
42ddf8b
Compare
|
Another heuristic for transitive dependency updates that @mctofu mentioned is whether the dependencies being updated have a mix of Since it is possible to have a transitive dependency update that only updates the transitive dependency, I think we can get away with checking if any of the dependencies being updated is not a |
Checks for a mix of top_level? and !top_level? dependencies to determine whether a PR updates a transitive dependency. Might be enough to just check for !top_level?
Nishnha
force-pushed
the
nishnha/transitive-pr-message
branch
from
42ddf8b to
062b377
Compare
mctofu
left a comment
mctofu
left a comment
There was a problem hiding this comment.
Looks good! Just had some minor cleanup suggestions.
|
I deployed this but haven't been able to generate a PR that uses the updated copy. Not sure if it's due to how Oddly enough, the dry-runner is showing the updated copy when I pass in the exact same job, so maybe this will only work for newly created PRs. |
|
Yeah, from #4652:
Implementation details in #4652 (comment), hopefully we can fix that at some point... |
3 participants
This PR adds a
#transitive_multidependency_intromethod to provide a more descriptive PR intro when multiple dependencies are updated in a transitive dependency update.Some caveats:
Avoids using the transitive multidependency message in lieu of the removed dependency message if any dependency is removed in an update.
Still uses the old PR message when a single transitive dependency is updated (see https://github.com/dsp-testing/dependabot-reproduce-update-not-possible/pull/66 for an example)
An example PR after this change would read as:
Also see the image below: